fix: don't crash on utf18 autocompletion #1129
Conversation
alexanderadam
force-pushed
the
fix/dont_crash_on_utf16_method_autocompletion
branch
from
1c50bc8 to
2759a0a
Compare
In TruffleRuby, when collecting symbols, some candidates can be nil. This adds a guard to skip nil candidates before trying to access their encoding property, preventing NoMethodError.
| def safe_grep(candidates, pattern) | ||
| target_encoding = Encoding.default_external | ||
| candidates.filter_map do |candidate| | ||
| next unless candidate |
There was a problem hiding this comment.
This part is not needed.
Array#grep accepts nil, but all candidates passed to this method is always an array of string.
candidates = something.collect{|m| m.to_s}
safe_grep(candidates, pattern)There was a problem hiding this comment.
I was surprised as well but without this the tests fail on TruffleRuby (compare the builds on the two commits in this PR).
It's working on every other platform though. 🤔
Or was I missing a bit here or misunderstanding something?
/CC @eregon
There was a problem hiding this comment.
Sorry, my comment "always an array of string" was wrong.
# Line 278
candidates = Symbol.all_symbols.collect do |s|
s.inspect
rescue EncodingError
# ignore (this part creates nil)
end
safe_grep(candidates, /^#{Regexp.quote(sym)}/)Changing this collect to filter_map to reject nil seems better.
| converted = candidate.encoding == target_encoding ? candidate : candidate.encode(target_encoding) | ||
|
|
||
| converted if pattern.match?(converted) | ||
| rescue Encoding::UndefinedConversionError, Encoding::InvalidByteSequenceError, Encoding::CompatibilityError, EncodingError |
There was a problem hiding this comment.
Encoding::UndefinedConversionError, Encoding::InvalidByteSequenceError and Encoding::CompatibilityError are all subclass of EncodingError.
Simply rescue EncodingError is enough, or rescue Encoding::CompatibilityError if match? is the only method called.
| candidates.filter_map do |candidate| | ||
| next unless candidate | ||
|
|
||
| converted = candidate.encoding == target_encoding ? candidate : candidate.encode(target_encoding) |
There was a problem hiding this comment.
Encoding conversion is more than safe-grep. I think this method should only do safe grep (match? and rescue) for simplicity.
Encoding conversion is already done in line 205.
def completion_candidates(preposing, target, postposing, bind:)
...
completion_data = retrieve_completion_data(target, bind: bind, doc_namespace: false).compact.filter_map do |i|
i.encode(Encoding.default_external)
rescue Encoding::UndefinedConversionError
# If the string cannot be converted, we just ignore it
nil
end
...
end
2 participants
fixes #52
PS: I'm looking for a new adventure in case anybody is looking to hire or work with a Ruby/Rails/Crystal dev
PPS: would you be so kind and add the
hacktoberfest-acceptedlabel to this issue in case you find that PR helpful? 🥺